Skip to content

add autoApproved status;#86

Merged
pashidlos merged 2 commits into
Visual-Regression-Tracker:masterfrom
vkostromin94:master
May 26, 2022
Merged

add autoApproved status;#86
pashidlos merged 2 commits into
Visual-Regression-Tracker:masterfrom
vkostromin94:master

Conversation

@vkostromin94

@vkostromin94 vkostromin94 commented May 23, 2022

Copy link
Copy Markdown
Contributor

When the response had an autoApproved status, Gson could not correctly parse the status on this method:

protected <T> T handleResponse(HttpResponse<String> response, Class<T> classOfT) {
        String responseBody = response.body();
        if (!String.valueOf(response.statusCode()).startsWith("2")) {
            throw new TestRunException(responseBody);
        }
        return gson.fromJson(responseBody, classOfT);
    }

This resulted in a further NPE in the method track.

@vkostromin94

vkostromin94 commented May 23, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

STR:

  1. Create a new project, enable the autoApprove feature
  2. Run a test on branch a (not base) with a new screenshot (with status new)
  3. Approve a new screenshot
  4. Run a test with the same screenshot in branch b

Actual result:

NPE in method track. Object testResultDTO contains status=null.

Details

image

pashidlos
pashidlos requested changes May 23, 2022
View reviewed changes

@pashidlos pashidlos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vkostromin94 good catch!
could you also add test for this case?

@vkostromin94

vkostromin94 commented May 25, 2022

Copy link
Copy Markdown
Contributor Author

@vkostromin94 good catch! could you also add test for this case?

No problem, done.

@vkostromin94
vkostromin94 requested a review from pashidlos May 25, 2022 19:45
pashidlos
pashidlos approved these changes May 26, 2022
View reviewed changes

@pashidlos pashidlos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@pashidlos
pashidlos merged commit c599671 into Visual-Regression-Tracker:master May 26, 2022
@pashidlos

pashidlos commented May 26, 2022

Copy link
Copy Markdown
Member

@vkostromin94 thank you for contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pashidlos pashidlos pashidlos approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vkostromin94 @pashidlos